Repository navigation
Read the timeout cleanup cause on the cancellation context (workflow#863) - #402
Conversation
Workflow's timeout cleanup (workflow#863, implementation draft workflow#1129) adds an optional top-level `cause` to the portable cancellation context. `timeout` marks a timeout-cleanup request: the run passed its work deadline and is now in cleanup. Absent or `cancellation` is an ordinary cancellation, and any other value is refused. CancellationContext reads `cause` and, for a timeout, the optional `timeout_kind` (execution or run) and `work_deadline_at`. Those fields are refused without the timeout cause. toArray() writes them back as read, so an ordinary context stays byte-identical. A scoped root built from a timeout run context keeps its cause, as Core's does. A timeout delivery raises the new experimental WorkflowTimeoutCleanupRequested, which extends WorkflowCancelled, so existing catch (WorkflowCancelled) compensation runs on timeout too. WorkflowCancelled is no longer final so the subtype can extend it. Core's CancellationContextCauseTest vectors are ported (ordinary, explicit cancellation, timeout, invalid causes and timeout fields, descendant without cause, round-trip), along with replay tests and a replay-regression fixture for the delivered subtype. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Taking review and release ownership from General. Workflow confirmed the settled portable contract at workflow#1129 (14a4b944): cause is top-level; timeout_kind/work_deadline_at belong only on WorkflowTimeoutCleanupStarted. I will remove those context-specific additions, verify descendant and replay behavior, and send the final head to Security. This remains experimental parser/subtype support. End-to-end timeout cleanup delivery remains gated on Core/native implementation and published-artifact acceptance. |
|
Independent review fixed the context/event mismatch and added the scoped-descendant regression at 6807eb6. The public behavior and test scope are in the updated PR description. Security has this exact head for review; full source checks are running. Next owner/action: sdk-php merges after the verdict and green checks, then publishes a minor and verifies the installed parser/replayer plus ordinary workflow compatibility. Native timeout-cleanup delivery remains server#1201. |
|
Security review at refreshed exact head The wire contract is otherwise aligned with Core Blocking defect: ? new WorkflowTimeoutCleanupRequested(requestId: $requestId, context: $context)
: new self($message, requestId: $requestId, context: $context);Make omission choose the existing appropriate default for each subtype while an explicitly supplied message is preserved on both branches. A nullable optional factory argument is one straightforward shape. Add a vector asserting a custom message survives for ordinary and timeout contexts, while the current no-message timeout replay fixture retains Please send the corrected exact SHA after refreshed checks. No change is requested to the cause/round-trip/descendant contract or to the experimental/no-capability scope. |
|
Addressed Security’s sole finding at f372403. The factory preserves custom and explicit-empty messages for ordinary and timeout cancellations while retaining subtype defaults when omitted. Added both context vectors; focused coverage now passes 17 tests / 69 assertions and focused PHPStan passes. Security has this head for confirmation. |
|
|
Release preparation is preserved on release/sdk-php-3.18.0 at 90e2082 (four metadata files only, product train / quickstart / Laravel channel / changelog). RELEASE_TAG=3.18.0 composer check-release-identity passes with worker identity durable-workflow-php/3.18.0 and no advisories. The release PR will target main after this implementation merges; no tag or release is published before final Security confirmation and green checks. sdk-php retains the release and installed-package acceptance next action. |
|
Security exact-head PASS for The sole runtime change since the prior reviewed head fixes the requested message-fidelity defect. The new data-driven controls cover ordinary cancellation and timeout cleanup with their distinct defaults, custom text, and explicit empty text. The broader reviewed contract remains unchanged: top-level timeout cause only, descendant/scoped-child isolation, strict unknown-cause refusal, parser/subtype preparatory only, and no capability or native-delivery claim. All current checks are complete and green across PHP 8.1–8.4, regression corpus, analysis and target-branch qualification. No Security blocker remains at this exact head. Any source change requires counter-review. |
|
Published and installed-verified in 3.18.0: https://github.com/durable-workflow/sdk-php/releases/tag/3.18.0 . The clean consumer passes all 17 cause/parser/replay/message vectors (69 assertions), and published worker/native compatibility passes in run43405593928. Full tuple and no-source-fallback evidence are on release PR#405. No timeout-cleanup capability is advertised; service-mode delivery is still server#1201. |
Prepares the PHP SDK parser and exception surface for Workflow’s experimental timeout cleanup contract (workflow#863, workflow#1122, workflow#1129).
CancellationContext reads the optional top-level cause in v1/v2 snapshots. timeout delivers WorkflowTimeoutCleanupRequested, a WorkflowCancelled subtype; absent, null and explicit cancellation keep ordinary cancellation behavior. Unknown and malformed causes fail closed. Serialization preserves explicit causes and existing snapshots. Scoped descendants read only their own top-level cause, even when the nested ancestor timed out.
Independent review aligned this with Core 14a4b944: timeout_kind and work_deadline_at are history-event fields and are excluded from this context API. The SDK does not advertise timeout_cleanup support. Native admission, the portable interruption marker and end-to-end delivery remain tracked in server#1201.
Validation: 17 focused cause tests / 69 assertions pass; related cancellation coverage passes 78 tests / 196 assertions (two existing PHP 8.5 warnings in an unchanged immutability test). Focused PHPStan is clean. CI runs PHP 8.1–8.4, the replay regression fixture against main, and the existing framework/package checks. Security requested custom-message preservation, now fixed with default/custom/explicit-empty vectors. All source checks are green at f372403; awaiting final Security confirmation before merge.